Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport of client: use RPC address and not serf after initial Consul discovery into release/1.5.x #16296

Conversation

hc-github-team-nomad-core
Copy link
Contributor

Backport

This PR is auto-generated from #16217 to be assessed for backporting due to the inclusion of the label backport/1.5.x.

The below text is copied from the body of the original PR.


Fixes #16211

Nomad servers can advertise independent IP addresses for serf and rpc. Somewhat unexpectedly, the serf address is also used for both Serf and server-to-server RPC communication (including Raft RPC). The address advertised for rpc is only used for client-to-server RPC. This split was introduced intentionally in Nomad 0.8.

When clients are using Consul discovery for connecting to servers, they get an initial discovery set from Consul and use the correct rpc tag in Consul to get a list of adddresses for servers. The client then makes a Status.Peers RPC to get the list of those servers that are raft peers. But this endpoint is shared between servers and clients, and provides the address used for Raft.

Most of the time this is harmless because servers will bind on 0.0.0.0 anyways., But in topologies where servers are on a private network and clients are on separate subnets (or even public subnets), clients will make initial contact with the server to get the list of peers but then populate their local server set with unreachable addresses.

Cluster administrators can work around this problem by using server_join with specific IP addresses (or DNS names), because the Node.UpdateStatus endpoint returns the correct set of RPC addresses when updating the node. So once a client has registered, it will get the correct set of RPC addresses.

This changeset updates the client logic to query Status.Members instead of Status.Peers, and then extract the correctly advertised address and port from the response body.


Notes:

  • The end-to-end testing required for this means it's going to just miss the 1.5.0 GA. This is probably safe to land in a patch release without worrying about backwards compatibility, because anyone with this topology is already using the server_join workaround described above.
  • This also closes fix wrong control plane discovery, when serf advertised address located in network, unreachable from clients #11895, which unfortunately got abandoned because we had some trouble understanding exactly what the problem was.
  • I've done some bench testing of this with a local development cluster, some extra IP addresses, and iptables rules.
  • I also stood it up on a real multi-host cluster using the changes in our E2E config from E2E: add multi-home networking to test infrastructure #16218 and everything seems to work as usual.
  • This is outside the code path of the non-Consul cloud discovery features, but I also tested it by disabling server join via Consul (by just breaking the consul config block) and used AWS EC2 auto-join via tags and that works just fine as well.

@hc-github-team-nomad-core hc-github-team-nomad-core force-pushed the backport/client-discover-by-members-rpc/possibly-knowing-bat branch from c6750da to f1179cd Compare March 2, 2023 18:37
@hc-github-team-nomad-core hc-github-team-nomad-core merged commit f7910da into release/1.5.x Mar 2, 2023
@hc-github-team-nomad-core hc-github-team-nomad-core deleted the backport/client-discover-by-members-rpc/possibly-knowing-bat branch March 2, 2023 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants